Masking out-of-source-domain data points for nearest neighbour remapping #317
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
for more information, see https://pre-commit.ci
huard
reviewed
Dec 12, 2023
Contributor
There was a problem hiding this comment.
I think this is a nice addition.
Couple of questions:
- Does this functionality have to be integrated in the Regridder? Could it be applied on the output by the user, or could we pre-compute the mask and pass the mask values to the Regridder?
- Is it reasonable to believe there might be other ways to generate a mask? If yes, then I believe it would make more sense to have a string argument for the mask generation method, that takes as input the method name, instead of a boolean.
Contributor
Author
|
@huard Thanks for your reply. I integrated it in the Regridder since creating the mask requires creating a Regridder object as well and so all the checks related to the dataset were required only once. However, since the Regridder initialisation then fails the linting checks with regards to the added complexity, I will have to move it to a separate method or function. I will try to find a general approach so it will be possible to later add further methods to generate masks. |
Contributor
|
Don't worry too much about the linter. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When remapping to a larger domain with the method
nearest_s2d, data points outside the original domain will - by the nature of this method - be extrapolated, i. e. have values from the edge of the original domain.This PR introduces a
Regridderoption to apply a "domain mask" that is generated by creating remapping weights with thebilinearmethod as an extra step (while havingunmapped_to_nanactivated) before creating the nearest neighbour remapping weights. This way, all data points that would count asunmappedfor thebilinearmethod will also be masked fornearest_s2d.This is useful when remapping data on regional grids such as CORDEX data. A demonstration can be seen here at the bottom of
Masking.ipynb:https://github.com/sol1105/xESMF/blob/domain_mask/doc/notebooks/Masking.ipynb
I added it as an option to the
Regridderbut the linting suggests that the__init__method gets too complex with this addition. I could also add this masking option as a separate function orRegriddermethod. But before putting more work into this I would welcome your general feedback (and if you welcome the general idea of this masking option, also specific feedback on how to preferably deal with the complexity issue).